Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-31292][CORE][SQL] Replace toSet.toSeq with distinct for readability #28062

Closed
wants to merge 1 commit into from

Conversation

sekikn
Copy link
Contributor

@sekikn sekikn commented Mar 28, 2020

What changes were proposed in this pull request?

This PR replaces the method calls of toSet.toSeq with distinct.

Why are the changes needed?

toSet.toSeq is intended to make its elements unique but a bit verbose. Using distinct instead is easier to understand and improves readability.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Tested with the existing unit tests and found no problem.

@kiszk
Copy link
Member

kiszk commented Mar 28, 2020

ok to test

@SparkQA
Copy link

SparkQA commented Mar 28, 2020

Test build #120536 has finished for PR 28062 at commit 92f871b.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

Copy link
Member

@maropu maropu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks fine to me cc: @srowen

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, minor but reasonable

@maropu maropu closed this in 0b237bd Mar 28, 2020
maropu pushed a commit that referenced this pull request Mar 28, 2020
…ility

### What changes were proposed in this pull request?

This PR replaces the method calls of `toSet.toSeq` with `distinct`.

### Why are the changes needed?

`toSet.toSeq` is intended to make its elements unique but a bit verbose. Using `distinct` instead is easier to understand and improves readability.

### Does this PR introduce any user-facing change?

No

### How was this patch tested?

Tested with the existing unit tests and found no problem.

Closes #28062 from sekikn/SPARK-31292.

Authored-by: Kengo Seki <sekikn@apache.org>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
(cherry picked from commit 0b237bd)
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
@maropu
Copy link
Member

maropu commented Mar 28, 2020

Thanks! Merged to master/3.0

@srowen
Copy link
Member

srowen commented Mar 29, 2020

I would just have merged to master at this time, but it is so trivial that almost surely won't matter

@maropu
Copy link
Member

maropu commented Mar 29, 2020

Yea, since too minor & trivial, I've merged it to 3.0, too. If any problem, plz revert it from 3.0.

sjincho pushed a commit to sjincho/spark that referenced this pull request Apr 15, 2020
…ility

### What changes were proposed in this pull request?

This PR replaces the method calls of `toSet.toSeq` with `distinct`.

### Why are the changes needed?

`toSet.toSeq` is intended to make its elements unique but a bit verbose. Using `distinct` instead is easier to understand and improves readability.

### Does this PR introduce any user-facing change?

No

### How was this patch tested?

Tested with the existing unit tests and found no problem.

Closes apache#28062 from sekikn/SPARK-31292.

Authored-by: Kengo Seki <sekikn@apache.org>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants